Skip to content

fix(lsp): cold terminal requests; vendor rust-analyzer VFS#301

Merged
hongjr03 merged 13 commits into
masterfrom
fix/lsp-request-readiness
Jul 15, 2026
Merged

fix(lsp): cold terminal requests; vendor rust-analyzer VFS#301
hongjr03 merged 13 commits into
masterfrom
fix/lsp-request-readiness

Conversation

@hongjr03

@hongjr03 hongjr03 commented Jul 15, 2026

Copy link
Copy Markdown
Member

Summary

  • Cold requests: while the workspace/VFS is cold, pull diagnostics and workspace symbols return protocol-valid empty responses instead of parking forever; diagnostics refresh once ready (when the client supports it).
  • Vendor rust-analyzer VFS (tinymist-style owned fork of vfs + vfs-notify): replace the self-grown implementation; pin upstream in crates/vfs/UPSTREAM.md.
  • r-a message shape: Progress carries config_version + LoadingProgress and gates readiness; Loaded / Changed carry only file batches (no content generation token). Only Changed can trigger manifest auto-reload.
  • Content model: Option<Vec<u8>> in VFS; line endings stay in the host map. Hardlink / path-identity redirects removed.
  • Notify alignment: notify 8.2; OS watcher behind feature server-file-watcher (default on). Dummy loader when the feature is off (wasm via default-features = false).
  • Config-side glob expansion: loader Directories is r-a-shaped (include / exclude / extensions only). Manifest exclude globs → directory prefixes in project-model; shallow source globs like rtl/*.svEntry::Files. FileSet still uses PathMatcher for source-root classification.

Root cause

  1. Cold diagnostic/symbol requests were deferred without a guarantee they would complete → non-terminal startup.
  2. Homegrown VFS/notify had grown past the abstraction level used by mature LSPs (identity redirects, heavier surfaces).
  3. Configuration scans could look like external edits and re-trigger reloads.
  4. Loader-level globs and OS watching were tangled into the VFS core.

Design

Cold path (dispatch/request.rs)

Request Cold behavior
textDocument/diagnostic Empty full report + defer refresh
workspace/diagnostic Empty report + defer refresh
workspace/symbol Empty flat list

On readiness: invalidate diagnostics and send workspace/diagnostic/refresh when supported.

VFS fork (crates/vfs)

Piece Policy
Core / notify Owned fork of rust-analyzer
No OS watch dummy::DummyHandle
Directories Prefix include/exclude + extensions only
FileSet PathMatcher / filters for source-root roles (vide)
Removed Hardlink identity, watcher manager / generation watcher readiness

Loader config (project-model)

  • Expand exclude patterns such as rtl/excluded/** → absolute exclude prefixes before set_config.
  • Expand non-** source globs at config time to file lists; keep ** patterns as recursive directory loads.

Review guide

  1. src/global_state/dispatch/request.rs — cold terminal fallbacks
  2. crates/vfs/ + UPSTREAM.md + dummy.rs
  3. src/global_state.rs — notify vs dummy spawn
  4. crates/project-model — prefix / shallow-glob expansion
  5. event_loop / process_changes — Progress readiness, Loaded vs Changed

Validation

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test -p vfs
  • cargo test -p project-model --lib
  • cargo test -p vide --lib -- process_changes::tests main_loop::tests
  • cargo check -p vide --no-default-features / cargo check -p vide-lsp-wasm

@github-actions

Copy link
Copy Markdown

Docs preview: https://vide.pascal-lab.net/preview/pr-301/

Server-side notify stays best-effort at rust-analyzer's abstraction level:
configuration scans report Loaded, later disk/invalidate events report
Changed, and OS watcher failures are logged rather than terminal for a
generation.

Remove the WatcherManager state machine, coverage revisions, ScanFailed,
and WatcherStatus protocol. Workspace readiness follows content-scan
Progress only, matching other LSPs that do not treat notify completeness
as a readiness invariant.
@hongjr03 hongjr03 changed the title fix(lsp): make startup readiness terminal fix(lsp): terminal cold requests and Loaded vs Changed Jul 15, 2026
hongjr03 added 3 commits July 15, 2026 18:54
Replace the self-grown vfs crate with an owned tinymist-style fork of
rust-analyzer's `vfs` + `vfs-notify`, pinned to commit 5300ee2665.

Adaptations for vide so far:
- map `paths`/`stdx` onto `utils`
- feature-gate the notify backend
- document upstream and fork policy in UPSTREAM.md

Hardlink identity is gone with the old Vfs implementation. Call sites
across hir/ide/host still need a Phase 1 cutover to the new API
(`Option<Vec<u8>>`, r-a Change shape, r-a loader Config).
Wire hir/ide/project-model and the LSP host to the vendored r-a VFS:

- content is Option<Vec<u8>>; line endings live in the host map
- loader Message matches r-a: Progress carries config_version and
  LoadingProgress; Loaded/Changed carry only files
- generation readiness is gated solely by Progress::Finished
- drop VFS hardlink identity and remap paths through FileId redirects
- keep SV PathMatcher/FileSet partitioning and SOURCE_FILE_EXTENSIONS
  as intentional fork deltas
- retain #301 cold-request terminal fallbacks and Loaded vs Changed
  provenance for workspace reload
Apply cargo fmt, satisfy clippy -D warnings, and update ide test
fixtures to ChangedFile::create/modify helpers after the API change.
@hongjr03 hongjr03 changed the title fix(lsp): terminal cold requests and Loaded vs Changed fix(lsp): cold terminal requests; vendor rust-analyzer VFS Jul 15, 2026
hongjr03 added 4 commits July 15, 2026 19:26
Continue r-a convergence on the request-readiness branch:

- Bump `notify` 6 → 8.2 (rust-analyzer pin)
- Add `dummy::DummyHandle` for builds without OS watching
- Feature `server-file-watcher` (default on); wasm uses
  `vide` with default-features off so notify is not linked
- Expand manifest exclude globs to directory prefixes in
  project-model; remove `exclude_globs` from loader::Directories
- Keep FileSet PathMatcher for source-root classification
CI failed on `sources = ["rtl/*.sv"]`: after removing loader globs,
recursive Directories under `rtl` incorrectly accepted nested files.

- Expand non-`**` source globs to `Entry::Files` at config time
- Keep `**` patterns as recursive directory loads with prefix excludes
- Write files in the shell-separator semantics test so expansion works
Source attribution lives in module docs; a separate upstream pin file is
not needed for this owned fork.
Keep short module docs and non-obvious behavior notes; drop fork
narratives, outdated identity comments, and redundant restatements.
@hongjr03 hongjr03 merged commit dcf1ccd into master Jul 15, 2026
23 checks passed
@hongjr03 hongjr03 deleted the fix/lsp-request-readiness branch July 15, 2026 13:57
hongjr03 added a commit that referenced this pull request Jul 15, 2026
FileId's field is private on master after #301; update tests and
silence clippy too_many_arguments on snapshot/key constructors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant